Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change pager scrolling behaviour, Fixes #562 #565

Merged
merged 3 commits into from
Jan 30, 2024

Conversation

tswfi
Copy link
Contributor

@tswfi tswfi commented Nov 28, 2023

Questions Answers
Description? Change how pager links scroll back to top of the product list. Also remove scrolling when changing facets
Type? improvement
BC breaks? no (maybe? one class changed?)
Deprecations? no
Fixed ticket? Fixes #562
Sponsor company
How to test? Change facet filters => should not scroll so you can easily select other facets too. Use pager in product listing => should scroll to top (pretty consistently even with firefox)

tleon
tleon previously approved these changes Nov 29, 2023
@florine2623 florine2623 self-assigned this Jan 4, 2024
Copy link

@florine2623 florine2623 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @tswfi ,

I have 2 different behaviors with your PR.

With Firefox, when changing/using filter, it doesn't scroll back to the top. So the customer can filter multiple attributes at the same time.

Screen.Recording.2024-01-04.at.11.29.06.mov

On Chrome, it always scrolls to the top.

Screen.Recording.2024-01-04.at.11.26.21.mov

The behavior should be consistent of both browsers. It should be like Firefox.
I have npm run build before testing on both browsers.

Could you check ? ^^
Thanks!

@florine2623 florine2623 removed their assignment Jan 4, 2024
@tswfi
Copy link
Contributor Author

tswfi commented Jan 4, 2024

Cannot replicate on OSX chrome, did you also clear your browser cache (or force refresh)?

image
Screen.Recording.2024-01-04.at.14.26.51.mov

There are some minor "jumps" as new things are added to the page (like the "remove this filtering facet" buttons above the products and "clear all" button above the facets, but those are outside of the scope).

@florine2623
Copy link

Yes, i have updated my chrome to version 120.0.6099.129, empty cache and hard reload 🤔

Maybe another @PrestaShop/qa-functional could test it. In any case, it works well with Firefox. It just needs to be checked one more time on Chrome.

@florine2623 florine2623 removed their assignment Jan 4, 2024
@SharakPL
Copy link
Contributor

SharakPL commented Jan 5, 2024

TBH I've never liked filters in the left column. It's too messed up. Would be much better to move it on top of the products list in the center column and categories block to the top.

@tswfi
Copy link
Contributor Author

tswfi commented Jan 6, 2024

TBH I've never liked filters in the left column. It's too messed up. Would be much better to move it on top of the products list in the center column and categories block to the top.

A setting would be the best.. Some merchants have lots of filters and having them on top of the products is not that easy to style nicely. Preferably there should be at least three different ways to show the filters. 1) like now 2) above the products 3) as a "sliding panel" from a button

Anyway, changing the logic is out of scope for this pull request.

@AureRita
Copy link

Hi @tswfi

Thank you for your PR, I tested it and it seems to works in my case as you can see :

recording.80.webm

Tested on develop and 8.1.x.
Because the PR seems to works as expected, It's QA ✔️

Thank you

@nicosomb nicosomb added this to the Beta milestone Jan 30, 2024
@nicosomb nicosomb merged commit 8412f7a into PrestaShop:develop Jan 30, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Really strange scrolling issue when paging with firefox
10 participants